Skip to content

Use JsonPointer syntax - #348

Open
usku01 wants to merge 20 commits into
mainfrom
feature/#344-use-json-pointer-syntax
Open

Use JsonPointer syntax#348
usku01 wants to merge 20 commits into
mainfrom
feature/#344-use-json-pointer-syntax

Conversation

@usku01

@usku01 usku01 commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@usku01
usku01 requested a review from a team June 19, 2026 08:40
@jpraet

jpraet commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

JsonPointer syntax only needs to be used for InEnum.BODY.
A query, path or header param with name "foo.bar" should be accepted.

@usku01

usku01 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

JsonPointer syntax only needs to be used for InEnum.BODY. A query, path or header param with name "foo.bar" should be accepted.

What if we have a query parameter which would be a list, shouldn't we add the index in the name? And what I mean by that is of the query param is named 'parameter.id', if we have an issue with the second member of the list, then the name in the issue should be 'parameter.id/1'?

@usku01

usku01 commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator Author

JsonPointer syntax only needs to be used for InEnum.BODY. A query, path or header param with name "foo.bar" should be accepted.

What if we have a query parameter which would be a list, shouldn't we add the index in the name? And what I mean by that is of the query param is named 'parameter.id', if we have an issue with the second member of the list, then the name in the issue should be 'parameter.id/1'?

I have applied changes to the implementation to allow "." in name

@jpraet

jpraet commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

JsonPointer syntax only needs to be used for InEnum.BODY. A query, path or header param with name "foo.bar" should be accepted.

What if we have a query parameter which would be a list, shouldn't we add the index in the name? And what I mean by that is of the query param is named 'parameter.id', if we have an issue with the second member of the list, then the name in the issue should be 'parameter.id/1'?

I think it's fine to keep using parameter.id[1] there.
JSON Pointer is specifically for pointing to a specific value within a JSON object.

@usku01
usku01 requested a review from jpraet June 26, 2026 09:15
@sonarqubecloud

Copy link
Copy Markdown

@sonarqubecloud

Copy link
Copy Markdown

@usku01
usku01 requested a review from jpraet July 30, 2026 08:03
public class JsonPointerUtil {

// e.g: /, field, /field, /field/0, /field/0/nested
private static final Pattern JSON_POINTER_PATTERN = Pattern.compile("/+[a-zA-Z0-9-]*+(/[a-zA-Z0-9-]++)*+");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we are too strict with this regex (e.g. allowing underscores).

Also, technically the empty string "" is also a valid JSON Pointer. It can be used to point to the body itself.

Currently empty string will be converted to null in transformName.

int index = 0;
for (String ssin : ssins.getValue()) {
ssin(new Input<>(ssins.getIn(), ssins.getName() + "[" + index + "]", ssin));
String name = JsonPointerUtil.transformName(ssins.getIn(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we remove the transformName call here?

We want auto-conversion with a warning when this is called with Input.body("ssins", myList); instead of "/ssins" no? Same for the refData methods below.

String indexFormat = ProblemConfig.isJsonPointerEnabled() && in == InEnum.BODY ? ("/" + source.indexOf(value))
: ("[" + source.indexOf(value) + "]");
String nameWithIndex = name + indexFormat;
return referencedResourceNotFound(in, JsonPointerUtil.transformName(in, nameWithIndex), value);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, shouldn't we remove the transformName call and rely on auto-conversion with warning instead?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants